This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
Add ReadOnlySequence support to Utf8JsonReader with unit tests #33462
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bartonjs
reviewed
Nov 13, 2018
src/System.Text.Json/src/System/Text/Json/Utf8JsonReader.MultiSegment.cs
Show resolved
Hide resolved
ahsonkhan
changed the title
stephentoub
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/CustomUncheckedBitArray.cs
Outdated
Show resolved
Hide resolved
stephentoub
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/CustomUncheckedBitArray.cs
Outdated
Show resolved
Hide resolved
stephentoub
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/CustomUncheckedBitArray.cs
Outdated
Show resolved
Hide resolved
stephentoub
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/CustomUncheckedBitArray.cs
Outdated
Show resolved
Hide resolved
stephentoub
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/CustomUncheckedBitArray.cs
Outdated
Show resolved
Hide resolved
stephentoub
reviewed
Nov 14, 2018
stephentoub
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/Utf8JsonReader.MultiSegment.cs
Show resolved
Hide resolved
stephentoub
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/Utf8JsonReader.MultiSegment.cs
Show resolved
Hide resolved
stephentoub
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/Utf8JsonReader.MultiSegment.cs
Outdated
Show resolved
Hide resolved
stephentoub
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/Utf8JsonReader.MultiSegment.cs
Show resolved
Hide resolved
bartonjs
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/Utf8JsonReader.MultiSegment.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/Utf8JsonReader.MultiSegment.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/Utf8JsonReader.MultiSegment.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Nov 14, 2018
src/System.Text.Json/src/System/Text/Json/Utf8JsonReader.MultiSegment.cs
Outdated
Show resolved
Hide resolved
|
Any other feedback (other than what's already pending)? |
ahsonkhan
changed the title
|
If there is no other significant PR feedback, I would like to merge this in so it can be consumed upstream. I would prefer to address non-blocking issues separately. |
jlennox
pushed a commit
to jlennox/corefx
that referenced
this pull request
Dec 16, 2018
…t#33462) * Use a custom BitArray impl instead of a stack for memory efficiency * Grow the bit array by doubling and apply BitArray PR feedback * Add ReadOnlySequence ctor and fields to support multi-segment * Update the ref by auto-generating it * Add valid JSON multi-segment tests and update ReadMultiSegment to use GetNextSpan * Address previous PR feedback around comments * Address PR feedback and add some invalid json tests. * Update invalid json tests and fix test returned byte position in line. * Fix exception messages and copy only what is relevant to the stack span. * Add tests which compares span-sequence state during reading and fix bugs. * Remove unused variable and update ROSequence tests. * Fix changes related to BitStack. * Remove previous bit array impl in favor of BitStack.
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
…t/corefx#33462) * Use a custom BitArray impl instead of a stack for memory efficiency * Grow the bit array by doubling and apply BitArray PR feedback * Add ReadOnlySequence ctor and fields to support multi-segment * Update the ref by auto-generating it * Add valid JSON multi-segment tests and update ReadMultiSegment to use GetNextSpan * Address previous PR feedback around comments * Address PR feedback and add some invalid json tests. * Update invalid json tests and fix test returned byte position in line. * Fix exception messages and copy only what is relevant to the stack span. * Add tests which compares span-sequence state during reading and fix bugs. * Remove unused variable and update ROSequence tests. * Fix changes related to BitStack. * Remove previous bit array impl in favor of BitStack. Commit migrated from dotnet/corefx@0354aa6
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TODO:
Improve multi-segment test coverage (particularly invalid JSON)https://github.com/dotnet/corefx/issues/33584Refactor duplicate code between single segment and multi-segment (where possible)https://github.com/dotnet/corefx/issues/33585